Skip to content

Conversation

comex
Copy link
Contributor

@comex comex commented Mar 12, 2018

  • The bootstrap crate currently passes -v to Cargo if itself invoked with -vv. But Cargo supports -vv (to show build script output), so make bootstrap pass that if itself invoked with -vvv. (More specifically, pass N '-v's to Cargo if invoked with N+1 of them.)

  • bootstrap.py currently tries to pass on up to two '-v's to cargo when building bootstrap, but incorrectly ('-v' is marked as 'store_true', so argparse stores either False or True, ignoring multiple '-v's). Fix this, allow passing any number of '-v's, and make it consistent with bootstrap's invocation of Cargo (i.e. subtract one from the number of '-v's).

  • Also improve bootstrap.py's config.toml 'parsing' to support arbitrary verbosity levels, + allow command line to override it.

@pietroalbini pietroalbini added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 12, 2018
@pietroalbini
Copy link
Member

Highfive failed to assign a reviewer.

r? @Mark-Simulacrum

@Mark-Simulacrum
Copy link
Member

r=me after fixing tidy:

[00:06:25] tidy error: /checkout/src/bootstrap/flags.rs:32: line longer than 100 chars

@kennytm kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 13, 2018
@@ -597,10 +597,8 @@ def build_bootstrap(self):
self.cargo()))
args = [self.cargo(), "build", "--manifest-path",
os.path.join(self.rust_root, "src/bootstrap/Cargo.toml")]
if self.verbose:
for _ in range(0, max(0, self.verbose - 1)):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In both Python 2 and 3, range(0, self.verbose - 1) will be empty when self.verbose - 1 is zero or negative, so the max(0, _) is unnecessary.

@@ -29,7 +29,7 @@ use cache::{Interned, INTERNER};

/// Deserialized version of all flags for this compile.
pub struct Flags {
pub verbose: usize, // verbosity level: 0 == not verbose, 1 == verbose, 2 == very verbose
pub verbose: usize, // verbosity level: 0 == not verbose, 1 == verbose, 2 == very verbose, 3 == omg so verbose
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤣

(This line is >100 chars BTW)

pub fn is_very_verbose(&self) -> bool {
self.verbosity > 1
pub fn cargo_verbosity_level(&self) -> usize {
if self.verbosity == 0 { 0 } else { self.verbosity - 1 }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be simply self.verbosity.saturating_sub(1)

Alternatively you could use this in src/bootstrap/builder.rs to entirely get rid of this function.

// if rustbuild is invoked with N `-v`s, we pass N-1 `-v`s to cargo.
for _ in 1..self.verbosity {
    cargo.arg("-v");
}

- The bootstrap crate currently passes -v to Cargo if itself invoked
with -vv.  But Cargo supports -vv (to show build script output), so make
bootstrap pass that if itself invoked with -vvv.  (More specifically,
pass N '-v's to Cargo if invoked with N+1 of them.)

- bootstrap.py currently tries to pass on up to two '-v's to cargo when
building bootstrap, but incorrectly ('-v' is marked as 'store_true', so
argparse stores either False or True, ignoring multiple '-v's).  Fix
this, allow passing any number of '-v's, and make it consistent with
bootstrap's invocation of Cargo (i.e. subtract one from the number of
'-v's).

- Also improve bootstrap.py's config.toml 'parsing' to support arbitrary
verbosity levels, + allow command line to override it.
@comex
Copy link
Contributor Author

comex commented Mar 16, 2018

Updated in light of review comments.

@Mark-Simulacrum
Copy link
Member

r? @kennytm

@kennytm
Copy link
Member

kennytm commented Mar 16, 2018

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 16, 2018

📌 Commit ec49234 has been approved by kennytm

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 16, 2018
@alexcrichton
Copy link
Member

@bors: rollup

kennytm added a commit to kennytm/rust that referenced this pull request Mar 17, 2018
Support extra-verbose builds

- The bootstrap crate currently passes -v to Cargo if itself invoked with -vv.  But Cargo supports -vv (to show build script output), so make bootstrap pass that if itself invoked with -vvv.  (More specifically, pass N '-v's to Cargo if invoked with N+1 of them.)

- bootstrap.py currently tries to pass on up to two '-v's to cargo when building bootstrap, but incorrectly ('-v' is marked as 'store_true', so argparse stores either False or True, ignoring multiple '-v's).  Fix this, allow passing any number of '-v's, and make it consistent with bootstrap's invocation of Cargo (i.e. subtract one from the number of '-v's).

- Also improve bootstrap.py's config.toml 'parsing' to support arbitrary verbosity levels, + allow command line to override it.
bors added a commit that referenced this pull request Mar 17, 2018
Rollup of 8 pull requests

- Successful merges: #48943, #48960, #48983, #49055, #49057, #49077, #49082, #49083
- Failed merges:
@bors bors merged commit ec49234 into rust-lang:master Mar 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants